Skip to content

fix(chat): stop chats storing a resource they can never send with - #6344

Merged
waleedlatif1 merged 4 commits into
stagingfrom
fix/chat-unaddressable-resource-id
Aug 6, 2026
Merged

fix(chat): stop chats storing a resource they can never send with#6344
waleedlatif1 merged 4 commits into
stagingfrom
fix/chat-unaddressable-resource-id

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • A chat resource persisted with id: "" made every later message fail. The write contract accepted z.string() while the send schema required .min(1), so POST /api/mothership/chat 400d before a stream existed and the client's reconnect 404d — the "stream death" symptom. The tab couldn't be removed either, since the delete route requires a non-empty id. 12 production chats are in this state (staging logs + copilot_chats.resources confirm it).
  • Origin: an agent-written file chip carries only a filename, and the client filled the missing id with '' when the file wasn't in its cached list — which it never is for a file the agent just created. Prod logs show the same file added with id: "", then 4s later with its real id.
  • Model the unresolved state (WorkspaceResourceRef) instead of faking an id, and resolve chip refs at one choke point that is allowed to refuse.
  • Close the stale-cache race by fetching the file list before giving up, so clicking a just-created file opens it instead of silently doing nothing.
  • Reject blank ids at the stream, write and send boundaries, and drop them wherever stored resources are read — which self-heals the affected chats with no migration.
  • Collapse the 5-6 duplicate POST /chat/resources every resource add was firing.
  • Log rejected chat bodies; a 400 previously left no trace anywhere, which is why this went unnoticed since June.

Type of Change

  • Bug fix

Testing

tsc clean, check:api-validation + check:react-query pass, 1571 copilot/API + 1787 workspace + 275 hooks tests pass. New unit tests cover ref resolution (incl. ambiguous-name refusal), the blank-id stream payload, and the sanitizer; each was verified to fail without its fix. Not yet exercised in a browser.

Notes for review

  • Two things ride along that aren't the blank-id bug: the duplicate-POST dedup and the wsres link id/path classification. Both are in the same subsystem and were found while tracing it.
  • Follow-up not taken here: resolution is file-shaped, but tables/workflows have the same "named before the client knows it exists" window. The per-type resource registry is the right home for it.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

A chat resource persisted with a blank id made every later message fail:
the write contract accepted `id: ''` while the send schema required
`min(1)`, so the request 400d before a stream existed and the client's
reconnect 404d. The tab could not be removed either, since the delete
route requires a non-empty id. Twelve production chats were in this state.

The id came from an agent-written file chip that carried only a filename:
the client filled the missing id with `''` when the file was absent from
its list, which it always is for a file the agent just created.

- model the unresolved state (`WorkspaceResourceRef`) instead of faking an
  id, and resolve chip refs at one choke point that may refuse
- close the stale-cache race by fetching the file list before giving up,
  so clicking a just-created file opens it instead of doing nothing
- reject blank ids at the stream, write and send boundaries, and drop them
  wherever stored resources are read, which self-heals affected chats
- collapse the 5-6 duplicate POSTs every resource add was firing
- log rejected chat bodies, which previously left no trace at all
@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 6, 2026 10:09pm

Request Review

@cursor

cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches chat send validation, resource persistence, and stream handling across API and client; changes are defensive but affect core mothership chat flows and stored chat data shape.

Overview
Fixes chats that could persist a resource with an empty id (often from a filename-only file chip before the file list caught up), which then blocked every later send and made the tab impossible to delete.

Modeling and resolution: Introduces WorkspaceResourceRef for unresolved chip references instead of inventing id: ''. resolveWorkspaceResourceRef centralizes file matching (id, VFS path, unique name) and refuses ambiguous names. Home resolves before opening a tab, refetches workspace files with staleTime: 0 when needed, and surfaces toast errors instead of silent failure or broken tabs.

Defense in depth: hasAddressableId / isAddressableResource and sanitizeChatResources (replacing canonicalizeDesktopSessionResources at persistence boundaries) drop blank ids on read/write. Rejection is added at chat send (dropUnaddressableAttachments), resource API schema, and stream resource payloads. addResource ignores unaddressable resources; duplicate POST persistence for the same resource in one tick is deduped via in-flight/pending keys.

Related UI fixes: wsres-click forwards id/path without collapsing to empty id; shared findWorkspaceFileByPath for path resolution.

Reviewed by Cursor Bugbot for commit 6bea695. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR prevents unresolved or blank-ID chat resources from being persisted or sent, sanitizes existing resource lists, and resolves file chips against a refreshed workspace-file list.

  • Introduces an explicit unresolved resource-reference type and centralized file-reference resolution.
  • Enforces addressable resource IDs at stream, API, persistence, and send boundaries.
  • Deduplicates concurrent resource persistence and adds validation and sanitizer coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts Adds centralized resolution of lossy file references while refusing ambiguous or unaddressable resources.
apps/sim/app/workspace/[workspaceId]/home/home.tsx Resolves resource chips against cached files, retries after a forced refresh, and reports unresolved selections.
apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts Rejects unaddressable tabs, sanitizes restored resources, and deduplicates concurrent persistence requests.
apps/sim/lib/copilot/resources/types.ts Defines unresolved resource references and centralizes addressability, canonicalization, and sanitization.
apps/sim/lib/copilot/chat/post.ts Drops blank-ID attachments before validation, sanitizes newly persisted resources, and logs rejected requests.
apps/sim/lib/copilot/resources/persistence.ts Sanitizes resource lists during persistence and removal so legacy blank-ID records self-heal.
apps/sim/lib/copilot/request/session/contract.ts Rejects stream resource events whose identifiers are blank or whitespace-only.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Chip[Message resource chip] --> Ref[WorkspaceResourceRef]
  Ref --> Resolve[Resolve against cached workspace files]
  Resolve -->|Found| Open[Open addressable resource]
  Resolve -->|Missing file| Refresh[Refetch workspace files]
  Refresh -->|Found| Open
  Refresh -->|Still missing| Refuse[Show error and refuse tab creation]
  Open --> Add[Deduplicated resource add]
  Add --> Persist[Sanitize and persist]
  Persist --> Send[Sanitize and send chat request]
Loading

Reviews (4): Last reviewed commit: "fix(chat): do not report an unreachable ..." | Re-trigger Greptile

Comment thread apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts
A rendered link collapses a resource's id and path into one href, so the
click handler cannot tell them apart. Classifying on a separator got a
bare filename in `path` wrong, and the resolver then trusted it as an id
— opening and persisting a tab pointing at nothing.

Drop the classifier and let the resolver try each candidate as an id, a
VFS path and a unique name. A file ref must now match a record the
workspace actually has; the stale-list case is covered by the refetch,
so an id that never resolves was never an id.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 9b69b03. Configure here.

The chip renders as a button with a hover state, so refusing to open it
silently reads as a broken control. Say what happened instead.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 791c098. Configure here.

A failed refetch and a successful one that found nothing were both
collapsed to an empty list, so a network blip told the user the file does
not exist. Keep the two apart and say which happened.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 6bea695. Configure here.

@waleedlatif1
waleedlatif1 merged commit 9ba51f9 into staging Aug 6, 2026
5 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/chat-unaddressable-resource-id branch August 6, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant